Skip to content

Introduce including user provided first-boot config at build time#8692

Merged
igorpecovnik merged 1 commit intomainfrom
sdk
Nov 12, 2025
Merged

Introduce including user provided first-boot config at build time#8692
igorpecovnik merged 1 commit intomainfrom
sdk

Conversation

@igorpecovnik
Copy link
Member

@igorpecovnik igorpecovnik commented Sep 29, 2025

Description

Inserting firstboot.conf from userpatches/ folder.

Documentation summary for feature / change

Config usage: https://docs.armbian.com/User-Guide_Autoconfig/

How Has This Been Tested?

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

Summary by CodeRabbit

  • New Features
    • Added support for user-provided firstboot configuration. When detected, the system displays an informational alert and applies the configuration before enabling desktop autologin.

@igorpecovnik igorpecovnik requested a review from a team as a code owner September 29, 2025 18:46
@igorpecovnik igorpecovnik added the Work in progress Unfinished / work in progress label Sep 29, 2025
@github-actions github-actions bot added size/small PR with less then 50 lines 11 Milestone: Fourth quarter release labels Sep 29, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 29, 2025

Walkthrough

Adds a pre-autologin check in lib/functions/rootfs/distro-agnostic.sh to detect ${USERPATCHES_PATH}/firstboot.conf, log an info alert, and copy it to ${SDCARD}/root/.not_logged_in_yet if present. No public/exported declarations changed.

Changes

Cohort / File(s) Change summary
Rootfs firstboot handling
lib/functions/rootfs/distro-agnostic.sh
Add check for ${USERPATCHES_PATH}/firstboot.conf; if present, print informational message and copy it to ${SDCARD}/root/.not_logged_in_yet before enabling desktop autologin.

Sequence Diagram(s)

sequenceDiagram
    participant Installer
    participant Filesystem
    participant DesktopManager

    Note over Installer,Filesystem: On image preparation
    Installer->>Filesystem: check `${USERPATCHES_PATH}/firstboot.conf`
    alt file exists
        Filesystem-->>Installer: return file
        Installer->>Filesystem: copy to `${SDCARD}/root/.not_logged_in_yet`
        Installer->>Installer: log info alert
    else file missing
        Filesystem-->>Installer: no file
    end
    Installer->>DesktopManager: enable desktop autologin
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review focus: lib/functions/rootfs/distro-agnostic.sh logic order (check/copy before autologin) and path variable usage/quoting.

Poem

I hopped through scripts with a twitchy nose,
Found a firstboot tucked where the user goes.
I copied it safe, left a little note,
So on first boot the config can gently float.
— 🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: introducing support for including a user-provided first-boot config file at build time, which aligns with the raw summary showing addition of firstboot.conf handling.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sdk

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added Needs review Seeking for review Framework Framework components labels Sep 29, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
action.yml (1)

15-19: Consider splitting unrelated changes into separate PRs.

This PR's objective is to "Introduce including user provided first-boot config at build time," but these changes add release artifact cleanup functionality, which appears unrelated to firstboot configuration.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between badbc99 and 6a08b23.

📒 Files selected for processing (2)
  • action.yml (2 hunks)
  • lib/functions/rootfs/distro-agnostic.sh (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/functions/rootfs/distro-agnostic.sh (1)
.github/generate_CODEOWNERS.sh (1)
  • display_alert (6-6)
🔇 Additional comments (2)
action.yml (1)

215-215: LGTM!

The usage of inputs.armbian_release_clean for removeArtifacts is correct and properly defaults to "false".

lib/functions/rootfs/distro-agnostic.sh (1)

112-116: [scratchpad]

The destination path for firstboot.conf is correct.

The review comment incorrectly assumes that .not_logged_in_yet should be a separate marker file and that firstboot.conf should be copied to a different location.

Analysis of the armbian-firstlogin script shows that /root/.not_logged_in_yet serves a dual purpose:

  1. Its presence triggers the first-login logic (checked at line 613: if [[ -f /root/.not_logged_in_yet && -n $(tty) ]]; then)
  2. It contains bash variable definitions that are sourced (line 93: source "$fp_config") to configure the automated setup

The script validates and sources this file as a bash script (lines 87-93), loading PRESET_* variables. The preset-firstrun.sh extension demonstrates this pattern by writing configuration variables directly to .not_logged_in_yet (lines 4-51).

Therefore, overwriting the empty marker file created at line 110 with the user's firstboot.conf at line 115 is the intended behavior.

Likely an incorrect or invalid review comment.

@igorpecovnik igorpecovnik force-pushed the sdk branch 2 times, most recently from 8df8645 to 5a8c30d Compare September 29, 2025 18:58
@igorpecovnik igorpecovnik added the Needs Documentation New feature needs documentation entry label Sep 29, 2025
@igorpecovnik igorpecovnik added Ready to merge Reviewed, tested and ready for merge and removed Needs review Seeking for review Needs Documentation New feature needs documentation entry Work in progress Unfinished / work in progress labels Nov 12, 2025
@igorpecovnik igorpecovnik merged commit 758c57b into main Nov 12, 2025
1 check passed
@igorpecovnik igorpecovnik deleted the sdk branch November 12, 2025 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

11 Milestone: Fourth quarter release Framework Framework components Ready to merge Reviewed, tested and ready for merge size/small PR with less then 50 lines

Development

Successfully merging this pull request may close these issues.

1 participant